Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

See enhancement issue #128 #136

Merged
merged 1 commit into from
Jan 17, 2019
Merged

See enhancement issue #128 #136

merged 1 commit into from
Jan 17, 2019

Conversation

antaldaniel
Copy link
Contributor

I added more functionality compared to Przemyslaw's initial idea, however, at the expense of adding a further dependency on the rOpenSci package RefManageR.

I believe that it is a rather widely used package, and the enhancement pays of for reproducible research.

I added more functionality compared to Przemyslaw's initial idea, however, at the expense of adding a further dependency on the rOpenSci package RefManageR.

I believe that it is a rather widely used package, and the enhancement pays of for reproducible research.
@ghost ghost assigned antaldaniel Jan 17, 2019
@ghost ghost added the in progress label Jan 17, 2019
@antaldaniel
Copy link
Contributor Author

Ok, I pulled to devel.
I hope this is only a Travis issue, something I would like to learn more about, because I struggle with it with iotables.
Also, I will take another look at the comma issue, but if anybody has an idea, I think it would make things nicer.

@antagomir antagomir merged commit 496c461 into rOpenGov:devel Jan 17, 2019
@ghost ghost removed the in progress label Jan 17, 2019
@antagomir
Copy link
Member

Ah now I realized that eurostat had this travis failure already before this PR (see issue #129).

R CMD build / check --as-cran on command line reveals this:

Undocumented arguments in documentation object 'get_bibentry'
‘format’

Can you fix and PR to devel branch?

It seems that after this everything is OK.

@antaldaniel
Copy link
Contributor Author

It seems OK, however, my issue remains on travis and r-hub that the dependency of eurostat, sf does not build and that makes continuous integration impossible. (Same for my iotables package, which is at last really ready for more highlight, but I struggle with this, because it depends on eurostat).

Or do you have any workaround to deal with this?

I wonder if the sf dependency is necessary at all? It is related to creating choropleths, I guess, which is not a core functionality and could be a vignette topic. I generally find map creation a developing area where the best solutions change from year to year.

@antagomir
Copy link
Member

In fact map visualizations are a topic where we have been planning to split a separate pkg (eurostat_geospatial or something) with @muuankarski et al. I am OK with removing sf from dependencies if it does not affect core functionality.

We have listed all imports in R/firstlib.R - it seems that sf::st_read is needed in get_eurostat_geospatial.R. Otherwise these are not needed for functionality in the R folder. @muuankarski do you see any workaround so that we could remove sf::st_read ?

@antagomir
Copy link
Member

All right. It seems that we should indeed move the GIS aspects (and perhaps the more generic bibtex entry stuff) to another package (eurostat_geospatial?) to solve this. This has been planned quite some time already but not completed. I try to see if we can find time to do this by summer 2019, not sure if faster schedule is possible. A separate geo package would help to simplify dependency structures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants